HW4_Trofimov #9
HW4_Trofimov #9
Conversation
Add function calculating aminoacids frequencies
My part of the project
Add description for my functions to README.md
add main function
… into HW4_Trofimov
| - **get_pI** | ||
|
|
||
| ```python | ||
| calculate_pI('RAHP') -> "Sequence: RAHP. Isoelectric point of each aminoacid: [('R', 10.8), ('A', 6.0), ('H', 7.6), ('P', 6.3)]" |
There was a problem hiding this comment.
Как-то не сходятся названия функций
| @@ -0,0 +1,361 @@ | |||
| # importing necessary modules | |||
| import protein_dict as pd | |||
There was a problem hiding this comment.
Сокращенное имя pd классически используется для импорта библиотеки pandas. Вы ее еще не проходили, так что ничего страшного, но в будущем лучше избегать импортов с классическими именами для других библиотек
| @@ -0,0 +1,361 @@ | |||
| # importing necessary modules | |||
| import protein_dict as pd | |||
| from random import choice | |||
There was a problem hiding this comment.
👍 хорошо, что импортировали только нужную вам функцию
| from random import choice | ||
|
|
||
|
|
||
| AMINO_LETTERS = [ |
There was a problem hiding this comment.
Не очень вижу смысл в списке (см. ниже комментарий по поводу использования этой переменной, там совет использовать множества)
Используется эта переменная один раз, и то там лучше применять множества.
Преобразовать список в множество занимает какое-то количество времени, поэтому эту переменную сразу надо определять как множество, и ниже использовать уже как есть (я ниже показала вариант кода, где из списка делается множество, но эти два комментария связаны, так что учтите)
| AMINO_LETTERS = [ | |
| AMINO_LETTERS = set("ACDEFGHIKLMNPQRSTVWY") |
| returns True or False | ||
| """ | ||
| unique_chars = set(seq) | ||
| aminoacids = set(pd.aa_monoistopic_mass_dict.keys()) |
There was a problem hiding this comment.
😭😭😭 Вы же буквально выше определили список со всеми аминокислотами
Получается, вместо просто сделать множество из списка, вы вытаскиваете ключи из словаря, и их список преобразуете в множество... На пару операций больше, чем нужно
(оптимальнее было бы выше сделать сразу сет, офк)
| """ | ||
| unique_chars = set(seq) | ||
| aminoacids = set(pd.aa_monoistopic_mass_dict.keys()) | ||
| return bool(unique_chars <= aminoacids) |
There was a problem hiding this comment.
Методы сравнения множеств предпочтительнее
| return bool(unique_chars <= aminoacids) | |
| return unique_chars.issubset(aminoacids) |
There was a problem hiding this comment.
Привет! Большое спасибо за комменты! А почему такой вариант предпочтительнее? Он же в любом случае выдает bool
There was a problem hiding this comment.
Да, в любом случае будет bool (если что, за это баллы не снижались, разумеется)
В целом обе операции одинаково эффективны и по сути, и по времени, и в реальности можно использовать любую. Я предлагаю при возможности использовать методы множеств во-первых, для того, чтобы вы научились их использованию, во-вторых, потому что если у вас огромный код, и вы не всегда помните, какого типа данных ваши переменные, то методы сетов сразу делают это ясным при чтении кода (или например, если надо что-то дописать\переписать).
Оператор сравнения < и >, кстати, не на всех типах данных работают одинаково, как вы знаете, например, со строками там применяется другая логика
| if pI_values is None: | ||
| # Default pKa_values if not provided | ||
| pI_values = pd.aa_pI |
There was a problem hiding this comment.
Здорово, что предусмотрена возможность ввода пользователем своего варианта
| """ | ||
|
|
||
| if amino_acid_alphabet is None: | ||
| # Default pKa_values if not provided |
There was a problem hiding this comment.
Хм.... а что в матрице скоров делают pKa?
|
|
||
| if amino_acid_alphabet is None: | ||
| # Default pKa_values if not provided | ||
| amino_acid_alphabet = "ACDEFGHIKLMNPQRSTVWY" |
There was a problem hiding this comment.
Зачем, если есть AMINO_LETTERS?
Ладно, если сразу задать AMINO_LETTERS как множество, то еще в этом есть смысл, но у вас буквально есть глобальная переменная с той же информацией
| else: | ||
| aligned_seq1.append("-") | ||
| # check original case of amionacid in seq2 | ||
| if seq2[j - 1].isupper(): |
There was a problem hiding this comment.
Подумайте над этой строкой: тут все падает для некоторых тестов
Какие-то из возможных ситуаций не были учтены, что приводит к IndexError 😉
| for amino_acid in sequences: | ||
| # If the aminoacid has been already in: | ||
| if amino_acid in amino_acid_frequency: | ||
| amino_acid_frequency[amino_acid] += 1 | ||
| # If the aminoacid hasn't been already in: | ||
| else: | ||
| amino_acid_frequency[amino_acid] = 1 |
There was a problem hiding this comment.
Здорово, но еще проще написать это через defaultdict
| sequence = "".join(pd.aa_one_to_three_letter.get(aa) for aa in seq) | ||
| return sequence[:-1] |
There was a problem hiding this comment.
Лучше было бы в словаре задать трехбукванные обозначения без -, и добавить его в джойне
| sequence = "".join(pd.aa_one_to_three_letter.get(aa) for aa in seq) | |
| return sequence[:-1] | |
| return "-".join(pd.aa_one_to_three_letter.get(aa) for aa in seq) |
| """ | ||
| seq = seq.upper() | ||
| if is_protein(seq) is True: | ||
| mass = sum(pd.aa_monoistopic_mass_dict.get(aa) for aa in seq) |
|
|
||
| Output: | ||
| returns sequence of aminoacids | ||
| """ |
There was a problem hiding this comment.
Хорошо написан докстринг, жаль, что в ридми про случайный выбор кодона не отмечено
| returns sequence of aminoacids | ||
| """ | ||
| seq = seq.upper() | ||
| if is_protein(seq) is True: |
There was a problem hiding this comment.
Как-то много вложенности, как минимум, один уровень убирается обратной проверкой:
| if is_protein(seq) is True: | |
| if not is_protein(seq): | |
| raise ValueError("Sequence is not a protein, input should be a protein") |
| raise ValueError("Sequence is not a protein, input should be a protein") | ||
|
|
||
|
|
||
| def main(*args: str): |
There was a problem hiding this comment.
Операцию над последовательностью в идеале можно сделать именованным аргументом, и тогда можно обойтись без усложнения в виде распаковки)
| action_list = { | ||
| "get_pI": get_pI, | ||
| "needleman_wunsch": needleman_wunsch, | ||
| "build_scoring_matrix": build_scoring_matrix, | ||
| "calculate_aa_freq": calculate_aa_freq, | ||
| "translate_protein_rna": translate_protein_rna, | ||
| "convert_to_3L_code": convert_to_3L_code, | ||
| "protein_mass": protein_mass, | ||
| } |
| raise ValueError("Error in number of sequences") | ||
|
|
||
| for sequence in args[:-1]: | ||
| if not all([letter.capitalize() in AMINO_LETTERS for letter in sequence]): |
There was a problem hiding this comment.
Ой, то есть мы итерируемся для каждого сиквенса по каждой его аминокислоте и по всем возможным аминокислотам ?(( Долго и неоптимально
Множествами это сделать было бы быстрее
| if not all([letter.capitalize() in AMINO_LETTERS for letter in sequence]): | |
| if not set(sequence).issubset(set(AMINO_LETTERS)): |
|
Неплохая работа! README:
Код:
Итог:
Сумма 8 |
No description provided.